Skip to content

Conversation

ScriptedAlchemy
Copy link
Contributor

@ScriptedAlchemy ScriptedAlchemy commented Sep 9, 2025

Description

ConsumeSharedModule does not properly populate its buildMeta and buildInfo properties during its build. This can cause critical issues in shared nested module scenarios where packages have complex dependency structures with nested package.json files (with ESM "type": "module" exports) as seen in #3779 with @mui/styled-engine.

Root cause

Bundlers rely on this metadata to understand Module characteristics such as module type detection (ESM vs CommonJS) and import/export statement generation. In the case of webpack and @mui/styled-engine, without the proper buildMeta info, it generates incorrect import statements, leading to runtime errors like:

TypeError: _emotion_styled__WEBPACK_IMPORTED_MODULE_0__ is not a function

Related Issue

module-federation/core#3779

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation.

- Add public get_options() method to ConsumeSharedModule for accessing private options field
- Fix type mismatches between Ustr and Identifier in module graph operations
- Add proper mutability declarations for module graph mutations
- Collapse nested if statements to follow clippy style guidelines
- Implement finishModules hook to copy build metadata from fallback modules
@Copilot Copilot AI review requested due to automatic review settings September 9, 2025 12:42
Copy link

netlify bot commented Sep 9, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit fd7e174
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68cdc1a0fb391100085d8562

@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Sep 9, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes compilation errors in the rspack_plugin_mf crate by resolving type mismatches and private field access issues in the consume shared plugin functionality.

  • Adds a new hook for processing ConsumeSharedModule instances during compilation to copy metadata from fallback modules
  • Introduces a public API method to access ConsumeSharedModule options that were previously private
  • Updates imports to include necessary types for module graph operations

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
crates/rspack_plugin_mf/src/sharing/consume_shared_plugin.rs Adds finish_modules hook implementation and updates imports to support module graph operations
crates/rspack_plugin_mf/src/sharing/consume_shared_module.rs Adds public getter method for accessing ConsumeOptions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Only process ConsumeSharedModule instances with fallback dependencies
if let Some(consume_shared) = module.as_any().downcast_ref::<ConsumeSharedModule>() {
// Check if this module has a fallback import
if consume_shared.get_options().import.is_some() {
Copy link
Preview

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition checks for import.is_some() but the logic processes modules regardless of whether they have fallback dependencies. Consider adding validation to ensure the module actually has fallback dependencies before adding it to the processing list.

Copilot uses AI. Check for mistakes.

Comment on lines 422 to 424
for module_id in consume_shared_modules {
let fallback_module_id = {
let module_graph = compilation.get_module_graph();
Copy link
Preview

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module graph is accessed multiple times within the loop (lines 424, 475, 487). Consider restructuring to minimize repeated access to the module graph for better performance.

Suggested change
for module_id in consume_shared_modules {
let fallback_module_id = {
let module_graph = compilation.get_module_graph();
let module_graph = compilation.get_module_graph();
for module_id in consume_shared_modules {
let fallback_module_id = {

Copilot uses AI. Check for mistakes.

Copy link
Contributor

github-actions bot commented Sep 9, 2025

📦 Binary Size-limit

Comparing fd7e174 to fix(browser): inject Buffer (#11714) by CPunisher

❌ Size increased by 8.00KB from 47.43MB to 47.44MB (⬆️0.02%)

- Add test configuration for nested consume shared module scenarios
- Include package dependencies and ESM/CJS module test setup
Copy link

codspeed-hq bot commented Sep 9, 2025

CodSpeed Performance Report

Merging #11621 will not alter performance

Comparing fix/consume-shared-plugin-compilation-errors (fd7e174) with main (7c0091e)

Summary

✅ 17 untouched

ScriptedAlchemy and others added 4 commits September 10, 2025 14:35
…raph access; clippy cleanups

- Process only modules with fallback import
- Compute fallback meta with single graph borrow
- Emit warning when fallback not found instead of defaulting
- Collapse nested ifs, use Option::map per clippy
@ScriptedAlchemy ScriptedAlchemy changed the title fix: resolve compilation errors in consume shared plugin feat: copy build meta onto share modules Sep 19, 2025
@github-actions github-actions bot added release: feature release: feature related release(mr only) and removed release: bug fix release: bug related release(mr only) labels Sep 19, 2025
@ScriptedAlchemy ScriptedAlchemy changed the title feat: copy build meta onto share modules Populate buildMeta and buildInfo on ConsumeSharedPlugin using fallbacks Sep 19, 2025
@github-actions github-actions bot removed the release: feature release: feature related release(mr only) label Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant